-
Notifications
You must be signed in to change notification settings - Fork 0
feat(actions): export reusable github actions checker #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds reusable GitHub Actions workflows (orchestrator and three child checks), Prettier config/ignore, package.json, .gitignore update, and a README heading correction. Changes
Sequence Diagram(s)%%{init: {"themeVariables":{"primaryColor":"#0b6efd33","secondaryColor":"#0b6efd22","noteBkg":"#f3f6ff","actorBorder":"#0b6efd"}}}%%
sequenceDiagram
autonumber
participant GitHub as "GitHub (PR event)"
participant Orchestrator as "pr-checks.yml (orchestrator)"
participant Runner as "Actions Runner"
participant Prettier as "prettier.yml"
participant Markdown as "markdown-check.yml"
participant Commitlint as "commit-check.yml"
GitHub->>Orchestrator: pull_request / workflow_call
Orchestrator->>Runner: evaluate event & inputs (is draft? run flags)
alt not draft & flags enabled
Orchestrator->>Prettier: workflow_call (inputs)
Orchestrator->>Markdown: workflow_call (inputs)
Orchestrator->>Commitlint: workflow_call (inputs)
Prettier->>Runner: checkout → setup Bun → install Prettier → run prettier --check
Markdown->>Runner: checkout → download+verify rumdl → run rumdl check
Commitlint->>Runner: checkout → setup Bun → install commitlint/config → ensure config → lint PR title/commits
Runner->>Orchestrator: child job results
else draft or disabled
Orchestrator->>Runner: skip child workflows
end
Orchestrator->>GitHub: set workflow status / outputs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🪛 LanguageToolREADME.md[uncategorized] ~26-~26: The official name of this software platform is spelled with a capital “H”. (GITHUB) 🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (4)
.github/workflows/prettier.yml (1)
40-44: Pin Prettier version or use project's version.Installing Prettier globally without a version constraint may lead to inconsistent behavior across workflow runs when new Prettier versions are released. Consider either:
- Pinning to a specific version:
pnpm add -g prettier@3.x.x- Installing from the project's package.json:
pnpm install && pnpm exec prettier --check .🔎 Option 1: Pin to a specific version
- - name: Install Prettier - run: pnpm add -g prettier + - name: Install Prettier + run: pnpm add -g prettier@3.4.2🔎 Option 2: Use project's Prettier version (recommended)
+ - name: Install dependencies + run: pnpm install + - name: Check formatting - run: prettier --check . + run: pnpm exec prettier --check .Remove the "Install Prettier" step entirely in this approach.
.github/workflows/markdown-check.yml (1)
14-14: Consider parameterizing the rumdl version.The hardcoded version (v0.0.185) will become outdated. Consider either:
- Using a workflow input to specify the version
- Pinning to a major version tag if available
- Documenting the version update process
.github/workflows/commit-check.yml (1)
24-25: Consider pinning commitlint version.Similar to Prettier, installing commitlint without version constraints may lead to breaking changes when new versions are released. Consider pinning to specific versions:
- run: pnpm add -g @commitlint/cli @commitlint/config-conventional + run: pnpm add -g @commitlint/cli@19.0.0 @commitlint/config-conventional@19.0.0.github/workflows/pr-checks.yml (1)
38-51: Consider simplifying conditional logic for clarity.The conditional expressions work correctly but are complex. The pattern
(inputs.run-X != false || github.event_name != 'workflow_call')effectively means:
- For direct PR triggers: always run (since inputs are only defined for workflow_call)
- For workflow_call triggers: respect the input
While functional, this could be clearer:
🔎 Alternative: Explicit handling by event type
prettier: needs: check-draft - if: needs.check-draft.outputs.should-run == 'true' && (inputs.run-prettier != false || github.event_name != 'workflow_call') + if: | + needs.check-draft.outputs.should-run == 'true' && + (github.event_name == 'pull_request' || inputs.run-prettier != false) uses: ./.github/workflows/prettier.ymlThis more explicitly shows that PR events always run while workflow_call respects inputs.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/commit-check.yml.github/workflows/markdown-check.yml.github/workflows/pr-checks.yml.github/workflows/prettier.yml
🔇 Additional comments (6)
.github/workflows/prettier.yml (1)
12-28: Thorough config detection.The Prettier configuration check covers all standard config file locations and package.json. The error message is clear and actionable.
.github/workflows/commit-check.yml (3)
10-12: Correct use of full history fetch.Using
fetch-depth: 0is appropriate here since the workflow needs to access commit messages between the base and head of the PR.
27-31: Good fallback strategy for missing config.Creating a default commitlint configuration ensures the workflow can run even in repositories without explicit commitlint setup, while still respecting custom configurations when present.
33-48: The conditionals usinggithub.event_name == 'pull_request'will work correctly.GitHub Actions properly inherits
github.event_namefrom the calling workflow in reusable workflows, and since these conditionals are at the job level (not workflow level), the context functions as expected. When pr-checks.yml is triggered by a pull_request event, github.event_name will correctly resolve to 'pull_request' in this workflow's jobs..github/workflows/pr-checks.yml (2)
3-22: Well-structured workflow inputs.The workflow provides flexible control over which checks to run while maintaining sensible defaults. The pull_request trigger types appropriately include
ready_for_reviewto handle draft PR transitions.
25-36: Draft PR detection logic is correct.The logic correctly skips checks for draft PRs while allowing checks to run for non-draft PRs and workflow_call invocations.
Time Submission Status
|
ed4fbbc to
0f97192
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.gitignore (1)
1-1: Consider adding common Node.js and pnpm artifacts.While
node_modulesis correctly ignored, consider enhancing this file with common entries that align with the pnpm/Node.js tooling being introduced:
- pnpm-specific:
pnpm-debug.log*,.pnpm-store/(if using workspace)- General logs:
*.log(catch npm/yarn logs too)- Environment files:
.env,.env.local- OS artifacts:
.DS_Store,Thumbs.db- IDE artifacts:
.vscode/,.idea/(optional, team preference)This is optional and depends on your project's scope, but it aligns with standard Node.js/pnpm hygiene.
🔎 Example enhanced .gitignore
+# Dependencies node_modules + +# Logs +*.log +pnpm-debug.log* +npm-debug.log* + +# Environment +.env +.env.local + +# OS +.DS_Store +Thumbs.db.github/workflows/prettier.yml (1)
40-41: Consider pinning the Prettier version for consistency.Installing Prettier without a version constraint may lead to inconsistent formatting checks over time as Prettier releases updates. Different workflow runs could use different Prettier versions, potentially causing unexpected failures.
🔎 Suggested approaches
Option 1: Pin to a specific version
- - name: Install Prettier - run: pnpm add -g prettier + - name: Install Prettier + run: pnpm add -g prettier@3.4.2Option 2: Install project dependencies if package.json exists (more flexible)
- name: Install Prettier - run: pnpm add -g prettier + run: | + if [ -f "package.json" ] && grep -q '"prettier"' package.json; then + pnpm install --frozen-lockfile + else + pnpm add -g prettier@3.4.2 + fiThen update the check command to use the local version:
- name: Check formatting - run: prettier --check . + run: npx prettier --check ..github/workflows/pr-checks.yml (1)
38-57: LGTM! The conditional logic correctly handles both event types.The workflow properly orchestrates the three check workflows, respecting both the draft PR status and selective check inputs. The conditional logic ensures checks run by default for direct PR events while honoring the input flags when called via
workflow_call.💡 Optional: Add clarifying comments
The conditional logic is correct but somewhat complex. Consider adding inline comments to explain the dual-mode behavior:
prettier: needs: check-draft + # Run if not draft AND (always for direct PR events OR input not explicitly disabled for workflow_call) if: needs.check-draft.outputs.should-run == 'true' && (inputs.run-prettier != false || github.event_name != 'workflow_call') uses: ./.github/workflows/prettier.yml
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
.github/workflows/commit-check.yml.github/workflows/markdown-check.yml.github/workflows/pr-checks.yml.github/workflows/prettier.yml.gitignore.prettierignore.prettierrcREADME.mdpackage.json
✅ Files skipped from review due to trivial changes (3)
- .prettierrc
- .prettierignore
- package.json
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/markdown-check.yml
- .github/workflows/commit-check.yml
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~26-~26: The official name of this software platform is spelled with a capital “H”.
Context: ...s by default. #### Basic Usage Create .github/workflows/pr-checks.yml in your reposi...
(GITHUB)
🔇 Additional comments (2)
.gitignore (1)
1-1: Correctly ignores Node.js dependencies.The
node_modulesentry is essential and properly placed for a Node.js project using pnpm.README.md (1)
1-156: LGTM! Comprehensive and well-structured documentation.The documentation clearly explains the available workflows, usage patterns, requirements, and configuration options. The capitalization fix in the title and the detailed examples will help users integrate these reusable workflows effectively.
Note: The static analysis hint about
.githubon line 26 is a false positive—.githubis a literal directory name and should remain lowercase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.github/workflows/pr-checks.yml (3)
45-47: Consider reordering the condition for better readability.The condition works correctly but could be more idiomatic by checking the event type first:
- if: - needs.check-draft.outputs.should-run == 'true' && (inputs.run-prettier != - false || github.event_name != 'workflow_call') + if: + needs.check-draft.outputs.should-run == 'true' && (github.event_name != + 'workflow_call' || inputs.run-prettier != false)This makes it clearer that checks always run for direct PR triggers and only respect the flag when called as a reusable workflow.
54-56: Consider reordering the condition for consistency.Same readability improvement as the prettier job for consistency:
- if: - needs.check-draft.outputs.should-run == 'true' && (inputs.run-markdown != - false || github.event_name != 'workflow_call') + if: + needs.check-draft.outputs.should-run == 'true' && (github.event_name != + 'workflow_call' || inputs.run-markdown != false)
61-63: Consider reordering the condition for consistency.Same readability improvement for consistency with other jobs:
- if: - needs.check-draft.outputs.should-run == 'true' && (inputs.run-commits != - false || github.event_name != 'workflow_call') + if: + needs.check-draft.outputs.should-run == 'true' && (github.event_name != + 'workflow_call' || inputs.run-commits != false)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/commit-check.yml.github/workflows/pr-checks.yml.github/workflows/prettier.ymlREADME.md
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/commit-check.yml
- .github/workflows/prettier.yml
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~26-~26: The official name of this software platform is spelled with a capital “H”.
Context: ...s by default. #### Basic Usage Create .github/workflows/pr-checks.yml in your reposi...
(GITHUB)
🔇 Additional comments (6)
README.md (2)
1-1: Capitalization fix looks good.The update from "Github Actions" to "GitHub Actions" is the correct spelling per the official GitHub branding.
3-190: Verify documentation against actual workflow implementations.The README comprehensively documents the reusable workflows and their usage. However, ensure that all documented specifications align with the actual workflow files:
- Input parameters (e.g.,
run-prettier,run-markdown,run-commits,package_json_file) and their defaults- Workflow behaviors (e.g., draft PR handling, pnpm version resolution from
package.json)- Requirements (e.g., Node.js 22+, pnpm defaults)
- File paths and configuration expectations
Since this PR adds the workflows themselves, please verify that the documented inputs, outputs, and default values match the
.github/workflows/pr-checks.yml,prettier.yml,markdown-check.yml, andcommit-check.ymlfiles being introduced..github/workflows/pr-checks.yml (4)
1-27: LGTM! Well-structured workflow configuration.The workflow triggers and input definitions are well-organized. The inputs provide sensible defaults that allow checks to run by default while offering flexibility for customization when called as a reusable workflow.
29-41: LGTM! Draft PR detection works correctly.The logic properly skips checks on draft PRs while allowing them to run when the PR is ready for review or when called as a reusable workflow.
66-66: Apply the same input handling fix as the prettier job.This has the same potential issue as line 50. Use a fallback to ensure proper default handling:
with: - package_json_file: ${{ inputs.package_json_file }} + package_json_file: ${{ inputs.package_json_file || 'package.json' }}
50-50: The code at line 50 of.github/workflows/pr-checks.ymlalready implements the defensive approach suggested in this review comment. The line currently reads:package_json_file: ${{ inputs.package_json_file || 'package.json' }}This is exactly the fallback pattern recommended in the review. Since
prettier.ymldefinespackage_json_filewith a default value of'package.json', the input handling is already properly guarded against empty or undefined values when triggered bypull_requestevents.No changes are needed.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (2)
package.json (1)
5-5: Consider removing themainfield.The
mainfield typically points to the entry point of a Node.js package. Since this appears to be a GitHub Actions repository rather than a publishable npm package, and noindex.jsfile is evident in the PR, this field is likely unnecessary.🔎 Proposed fix
"description": "", - "main": "index.js", "scripts": {.github/workflows/prettier.yml (1)
12-28: Consider making the config check more robust.The current check on line 24 uses
grep -q '"prettier"' "${{ inputs.package_json_file }}", which may produce false positives if "prettier" appears in other contexts (e.g., in a script name or dependency comment). Consider usingjqfor more precise JSON parsing.🔎 Proposed improvement
- name: Check for Prettier config run: | CONFIG_FILES=".prettierrc .prettierrc.json .prettierrc.js .prettierrc.yaml .prettierrc.yml .prettierrc.toml prettier.config.js prettier.config.mjs" HAS_CONFIG=false for file in $CONFIG_FILES; do if [ -f "$file" ]; then HAS_CONFIG=true break fi done - if [ "$HAS_CONFIG" = false ] && ! grep -q '"prettier"' "${{ inputs.package_json_file }}" 2>/dev/null; then + if [ "$HAS_CONFIG" = false ]; then + if [ ! -f "${{ inputs.package_json_file }}" ] || ! jq -e '.prettier // .devDependencies.prettier // .dependencies.prettier' "${{ inputs.package_json_file }}" > /dev/null 2>&1; then echo "::error file=.prettierrc::No Prettier configuration file found." echo "Please create a .prettierrc file or add prettier config to package.json" exit 1 + fi fi
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
.github/workflows/commit-check.yml.github/workflows/pr-checks.yml.github/workflows/prettier.ymlREADME.mdpackage.json
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/pr-checks.yml
- .github/workflows/commit-check.yml
🧰 Additional context used
🪛 actionlint (1.7.9)
.github/workflows/prettier.yml
13-13: property "package_json_file" is not defined in object type {}
(expression)
🪛 LanguageTool
README.md
[uncategorized] ~26-~26: The official name of this software platform is spelled with a capital “H”.
Context: ...s by default. #### Basic Usage Create .github/workflows/pr-checks.yml in your reposi...
(GITHUB)
🔇 Additional comments (3)
README.md (1)
1-1: LGTM!Correct capitalization of "GitHub Actions" following the official branding.
.github/workflows/prettier.yml (1)
30-33: LGTM! Bun setup is appropriate.Using Bun instead of pnpm is a valid choice and appears to be intentional per commit messages. However, ensure the README is updated to reflect this change (already flagged in README review).
package.json (1)
14-14: Prettier version 3.7.4 is valid and secure.Version 3.7.4 is the latest stable release with no known security vulnerabilities.
|
here are the results of testing in my personal public repo: 1. no prettier config and markdown lint error 2. prettier formatting error and markdown lint error 3. prettier formatting success, remaining markdown lint error 4. all success 5. all skipped |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (1)
.github/workflows/commit-check.yml (1)
24-28: Consider refactoring config detection for maintainability.The single-line conditional (line 26) checks multiple config formats but is difficult to read and maintain. Additionally, it's missing some valid commitlint config formats like
.commitlintrc.cjs,.commitlintrc.mjs, andcommitlint.config.js.🔎 Suggested refactor for better readability
- name: Create commitlint config if missing run: | - if [ ! -f .commitlintrc.json ] && [ ! -f .commitlintrc.js ] && [ ! -f .commitlintrc.yml ] && [ ! -f .commitlintrc.yaml ] && ! grep -q '"commitlint"' "package.json" 2>/dev/null; then + # Check for any existing commitlint configuration + if ! ls .commitlintrc* commitlint.config.* >/dev/null 2>&1 && \ + ! grep -q '"commitlint"' package.json 2>/dev/null; then + echo "No commitlint config found, creating default configuration..." echo '{"extends": ["@commitlint/config-conventional"]}' > .commitlintrc.json fiThis approach uses file globbing to detect any commitlint config file and is more maintainable.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/commit-check.yml.github/workflows/pr-checks-self.yml.github/workflows/pr-checks.yml.github/workflows/prettier.ymlREADME.md
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/pr-checks-self.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/prettier.yml
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~26-~26: The official name of this software platform is spelled with a capital “H”.
Context: ...s by default. #### Basic Usage Create .github/workflows/pr-checks.yml in your reposi...
(GITHUB)
🔇 Additional comments (6)
README.md (1)
1-1: LGTM!Correct capitalization of "GitHub" aligns with the official brand name.
.github/workflows/commit-check.yml (1)
30-35: LGTM!The PR title validation logic is clean and correct. The conditional execution ensures it only runs for pull request events, and piping the title to commitlint is the appropriate approach.
.github/workflows/pr-checks.yml (4)
3-20: LGTM!The workflow inputs are well-defined with appropriate defaults. The boolean flags provide good control over which checks execute.
22-34: LGTM!The draft PR detection logic is correct and properly outputs the
should-runstatus for downstream jobs. The conditional correctly skips checks only for draft PRs.
36-41: LGTM!The prettier job configuration is correct. The conditional logic properly respects both the draft status and the
run-prettierinput flag.
43-55: LGTM!The markdown and commits jobs follow the same correct pattern as the prettier job. The consistent structure makes the workflow easy to understand and maintain.
|
@zolotokrylin ready to review |
zolotokrylin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
None blocking comments. Up to you if you would like to resolve them in this or other PRs.









resolves https://github.com/holdex/pr-time-tracker-webhooks/issues/683
Summary by CodeRabbit
Chores
Style
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.